-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(server): Dry run always in client mode just for yaml manifest validation even with server side apply #564
fix(server): Dry run always in client mode just for yaml manifest validation even with server side apply #564
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #564 +/- ##
=======================================
Coverage 54.41% 54.42%
=======================================
Files 41 41
Lines 4798 4803 +5
=======================================
+ Hits 2611 2614 +3
- Misses 1974 1977 +3
+ Partials 213 212 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some minor comments.
Otherwise LGTM.
Tks a lot for working on this!
@anandf Please fix the DCO check |
…argoproj#548)" This reverts commit c0c2dd1. Signed-off-by: Anand Francis Joseph <[email protected]>
…side apply (argoproj#546)" This reverts commit 4a5648e. Signed-off-by: Anand Francis Joseph <[email protected]>
Signed-off-by: Anand Francis Joseph <[email protected]>
Signed-off-by: Anand Francis Joseph <[email protected]>
Signed-off-by: Anand Francis Joseph <[email protected]>
Signed-off-by: Anand Francis Joseph <[email protected]>
Signed-off-by: Anand Francis Joseph <[email protected]>
63a5a72
to
b77faaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Root Cause:
When server side apply is set to true, dry run is run in server mode. Dry run in server mode will fail if the target namespace is missing. So in cases where the sync option
CreateNamespace
is set to true, the dry run fails in server mode, and then attempts to re-run in client mode as a fallback mechanism. But the server side apply is still set. This is not a supported configuration inkubectl
when--dry-run
is set to client,--server-side
flag is not supported causing that to fail as well with the below error messageDue to this behaviour, when both
CreateNamespace=true
andServerSideApply=true
sync options are used, then the app sync fails while trying to validate the rendered yaml manifests.Solution
If it is a dry run, always run the dry run in client mode
--dry-run=client
even if the server side apply option is set--server-side
. Dry runs are used for validating if the rendered manifests satisfy the yaml correctness. For the actual sync, honour the--server-side
apply flag.Fixes
argoproj/argo-cd#16829
argoproj/argo-cd#13874
argoproj/argo-cd#16840